Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: improve logging #690

Closed
wants to merge 6 commits into from
Closed

chore: improve logging #690

wants to merge 6 commits into from

Conversation

lklimek
Copy link
Collaborator

@lklimek lklimek commented Sep 26, 2023

Issue being fixed or feature implemented

Logs are too verbose

What was done?

  1. Adjusted verbosity of some logs
  2. Implemented Logger.Log(level) that allows providing level as a variable
  3. Changed type of log levels from string to specialized type Level.

How Has This Been Tested?

Github Actions

Breaking Changes

None

Checklist:

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have added or updated relevant unit/integration/functional/e2e tests
  • I have made corresponding changes to the documentation

For repository code-owners and collaborators only

  • I have assigned this pull request to a milestone

@github-actions
Copy link

github-actions bot commented Oct 7, 2023

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@github-actions github-actions bot added the Stale label Oct 7, 2023
@lklimek lklimek removed the Stale label Oct 9, 2023
@lklimek lklimek added this to the v0.14.0 milestone Oct 17, 2023
@lklimek lklimek marked this pull request as ready for review October 19, 2023 08:17
@lklimek lklimek requested a review from shumkov October 19, 2023 08:18
Comment on lines -90 to -91
"height", stateData.Height,
"round", stateData.Round,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why did we remove height and round?

@@ -395,10 +395,10 @@ func (blockExec *BlockExecutor) ValidateBlockWithRoundState(
// Validate app info
if uncommittedState.AppHash != nil && !bytes.Equal(block.AppHash, uncommittedState.AppHash) {
return fmt.Errorf(
"wrong Block.Header.AppHash at state height %d, block %d. Expected %X, got %X",
"Block.Header.AppHash mismatch; state (or ABCI App) at height %d expects App Hash %X, block at height %d expects App Hash %X",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From my perspective, the new message is more confusing than the previous one. What if we just swap app hash params and keep the old one?

cmd/tenderdash/commands/light.go Outdated Show resolved Hide resolved
Copy link

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@github-actions github-actions bot added the Stale label Nov 10, 2023
@github-actions github-actions bot closed this Nov 14, 2023
@lklimek lklimek deleted the improve-logs-v14 branch February 1, 2024 13:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants